Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS: Hide Alerts due to navigation issue #423

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

ksharma-xyz
Copy link
Owner

@ksharma-xyz ksharma-xyz commented Dec 4, 2024

TL;DR

Removed service alert priority and restricted alert display to Android platform only.

What changed?

  • Renamed LocalPlatformTypeProvider to LocalAppPlatformProvider for clarity
  • Removed priority field from ServiceAlert data class
  • Added platform check to only show alerts on Android devices
  • Modified alert-related functions to handle alerts as List instead of Set
  • Added TODO comment explaining the iOS limitation with ComposeNavigation

How to test?

  1. Verify alerts are only visible on Android devices
  2. Confirm alerts still display correctly with heading and message
  3. Navigate through journey cards and check alert functionality
  4. Verify iOS app doesn't crash when handling alerts

Why make this change?

The service alert priority was unused and added unnecessary complexity. Additionally, iOS devices were crashing when navigating with HTML content as string arguments (see JetBrains issue CMP-7180). This change provides a temporary solution by limiting alerts to Android while maintaining app stability on iOS.

Copy link
Owner Author

ksharma-xyz commented Dec 4, 2024

@ksharma-xyz ksharma-xyz changed the title Alerts fix iOS UI: Rename platform provider and remove alert priority Dec 4, 2024
@ksharma-xyz ksharma-xyz changed the title UI: Rename platform provider and remove alert priority iOS: Hide Alerts due to navigation issue Dec 4, 2024
@ksharma-xyz ksharma-xyz marked this pull request as ready for review December 4, 2024 09:41
@ksharma-xyz ksharma-xyz added the iOS iOS Target label Dec 4, 2024 — with Graphite App
@ksharma-xyz ksharma-xyz changed the base branch from 12-04-add_isdebug_method_to_appplatform to graphite-base/423 December 4, 2024 09:47
@ksharma-xyz ksharma-xyz changed the base branch from graphite-base/423 to main December 4, 2024 09:48
@ksharma-xyz ksharma-xyz merged commit 40d8de5 into main Dec 4, 2024
2 checks passed
Copy link
Owner Author

Merge activity

  • Dec 4, 5:04 AM EST: A user merged this pull request with Graphite.

@ksharma-xyz ksharma-xyz deleted the 12-04-alerts_fix_ios branch December 4, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS iOS Target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant